fix(auth/middleware): handle numeric code field in auth error response#81
fix(auth/middleware): handle numeric code field in auth error response#81
Conversation
The auth service may return the 'code' field as a number (e.g. 401) instead of a string. This caused json.Unmarshal to fail with a type mismatch, surfacing as a 500 instead of propagating the actual error. Add unmarshalErrorResponse helper using json.RawMessage to tolerate both string and numeric code values in checkAuthorization and GetApplicationToken. X-Lerian-Ref: 0x1
WalkthroughThe changes add a new helper function 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
gandalf-at-lerian
left a comment
gandalf-at-lerian
left a comment
There was a problem hiding this comment.
LGTM. Clean fix for a subtle type mismatch bug.
The approach is correct: json.RawMessage defers decoding so you can attempt string first, then fall back to the numeric raw bytes — which produces the expected "401" string representation. Applied consistently to both checkAuthorization and GetApplicationToken.
The helper is well-factored — better than duplicating the same try-string-then-fallback logic in both call sites. ✅
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@auth/middleware/middleware.go`:
- Around line 52-83: unmarshalErrorResponse now accepts mixed JSON types for the
"code" field but lacks tests; add table-driven tests for unmarshalErrorResponse
covering cases: string code (e.g. "ERR"), numeric code (e.g. 401), null code,
missing code, empty string, and invalid JSON for code to ensure it returns the
expected commons.Response.Code (string or raw numeric string) and no unexpected
errors/500s. In the test file create subtests that call unmarshalErrorResponse
with representative bodies and assert returned resp.Code and error match
expected values; also add tests that exercise the middleware path(s) that call
unmarshalErrorResponse so end-to-end behavior is locked in.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6f7155dc-fcee-4cba-a12a-62aef4a56ad7
📒 Files selected for processing (1)
auth/middleware/middleware.go
| // unmarshalErrorResponse unmarshals a JSON response body into commons.Response, | ||
| // tolerating a numeric "code" field (the auth service may return code as a number). | ||
| func unmarshalErrorResponse(body []byte) (commons.Response, error) { | ||
| var raw struct { | ||
| EntityType string `json:"entityType,omitempty"` | ||
| Title string `json:"title,omitempty"` | ||
| Message string `json:"message,omitempty"` | ||
| Code json.RawMessage `json:"code,omitempty"` | ||
| } | ||
|
|
||
| if err := json.Unmarshal(body, &raw); err != nil { | ||
| return commons.Response{}, err | ||
| } | ||
|
|
||
| resp := commons.Response{ | ||
| EntityType: raw.EntityType, | ||
| Title: raw.Title, | ||
| Message: raw.Message, | ||
| } | ||
|
|
||
| if len(raw.Code) > 0 { | ||
| var code string | ||
| if err := json.Unmarshal(raw.Code, &code); err == nil { | ||
| resp.Code = code | ||
| } else { | ||
| // Numeric code — use raw representation (e.g. "401") | ||
| resp.Code = string(raw.Code) | ||
| } | ||
| } | ||
|
|
||
| return resp, nil | ||
| } |
There was a problem hiding this comment.
Add regression tests for mixed code types before merging.
This helper changes auth error parsing behavior in a critical middleware path, but there’s no test coverage for string/number/null/invalid code variants. Please add table-driven tests to lock this behavior and avoid reintroducing 500s on auth errors.
I can draft a complete table-driven _test.go for this helper and both call paths if you want.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@auth/middleware/middleware.go` around lines 52 - 83, unmarshalErrorResponse
now accepts mixed JSON types for the "code" field but lacks tests; add
table-driven tests for unmarshalErrorResponse covering cases: string code (e.g.
"ERR"), numeric code (e.g. 401), null code, missing code, empty string, and
invalid JSON for code to ensure it returns the expected commons.Response.Code
(string or raw numeric string) and no unexpected errors/500s. In the test file
create subtests that call unmarshalErrorResponse with representative bodies and
assert returned resp.Code and error match expected values; also add tests that
exercise the middleware path(s) that call unmarshalErrorResponse so end-to-end
behavior is locked in.
|
🎉 This PR is included in version 2.5.0-beta.9 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.5.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
3 participants
The auth service may return the 'code' field as a number (e.g. 401) instead of a string. This caused json.Unmarshal to fail with a type mismatch, surfacing as a 500 instead of propagating the actual error.
Add unmarshalErrorResponse helper using json.RawMessage to tolerate both string and numeric code values in checkAuthorization and GetApplicationToken.
X-Lerian-Ref: 0x1
Pull Request Checklist
Pull Request Type
Checklist
Please check each item after it's completed.
Additional Notes
Obs: Please, always remember to target your PR to develop branch instead of main.